Repository navigation
Stabilize inlay hints during incomplete edits - #787
Conversation
eitsupi
left a comment
eitsupi
left a comment
There was a problem hiding this comment.
This review was conducted with ChatGPT.
The overall approach looks sound and should significantly improve the issue reported in REditorSupport/vscode-R#1853 without disabling inlay hints.
I noticed one edge case in inlay_hint_apply_change() that may need fixing before merging:
- When an edit ends exactly at the start of a call, the call is considered untouched. For example, replacing
x <-withobj$inx <- target(a, b)producesobj$target(a, b). The cached hints may be preserved during a subsequent parse error, even though member calls are intentionally excluded from inlay hints. - The current boundary check only handles insertions at the call's start, not replacements ending there. Could you address this case and add a regression test?
Two non-blocking suggestions:
- Since incremental edits now update and reorder the cached call index, a quick typing-performance benchmark on a large file would be useful.
- Manual verification in VS Code would help confirm that the flickering reported in #1853 is sufficiently reduced.
Otherwise, the implementation and test coverage look promising.
eitsupi
left a comment
eitsupi
left a comment
There was a problem hiding this comment.
This review was conducted with ChatGPT.
The previous concern has been addressed. One additional non-blocking test suggestion:
Replacing x <- with obj$ in x <- target(a, b) leaves a space before target, producing obj$ target(a, b). Since the edit ends before the callee rather than exactly at its start, the cached hints may incorrectly survive a subsequent parse error.
A regression test covering this whitespace-separated boundary would be useful.
Temporary syntax errors while typing currently clear parameter-name inlay hints across the document, making unrelated lines shift as hints disappear and reappear. Preserve the last valid call index for untouched calls, move their UTF-16 positions with incremental edits, and refresh it after successful parsing.
Discard calls intersecting an edit and conservatively invalidate following calls when comments or strings may change lexical context. Invalidate calls when inserts, replacements, or deletions ending at or separated from the callee by spaces or tabs can change its name, namespace, or member context. Retain previously resolved parameter names during parse failures, honor current hint settings and viewport ranges, and restrict Quarto/R Markdown fallback hints to omitted R cells so fresh results take precedence.
Hints for the call being edited may still disappear until it parses successfully. Visual verification of flickering in VS Code remains outstanding.
Related: REditorSupport/vscode-R#1853
Validation:
$,@,::, renamed callees, multiline replacements, and deletion joining a call to a member prefix. These fail before the boundary fix and pass afterward; whitespace replacements still preserve hints. Added whitespace-separated prefix regressions for spaces, tabs, member and namespace prefixes, and UTF-16 positions, plus a check that intervening tokens preserve unrelated calls.stats::rnorm(10, 1, 2)calls, alternating insertion/deletion on the blank middle line. Median time was 2.530 ms per edit with the cached call-index update versus 0.080 ms without it, adding 2.450 ms per edit. The benchmark includes document content updates and keeps all 10,000 calls cached throughout; it excludes background parsing and editor rendering.git diff --checkpassed; no new lint findings in changed code.